Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(symbol): revert unique symbol in #5874 #6224

Merged
merged 2 commits into from
Apr 15, 2021
Merged

fix(symbol): revert unique symbol in #5874 #6224

merged 2 commits into from
Apr 15, 2021

Conversation

kwonoj
Copy link
Member

@kwonoj kwonoj commented Apr 12, 2021

This is manual revert to #5874.

Core team have discussed around #5919 for a while, and at this moment pretty much agreed

At least, providing as-is experience between v6 to v7 is something we'd like to have instead of making huge breaking changes. This doesn't mean we'll stay with this forever, it's just hard enough find right solution (and doesn't break whole world).

I'm making this PR to v7 not slip this accidentally when we publish official release.

Description:

Related issue (if exists):

Comment on lines 1 to 9
/** Symbol.observable addition */
/* Note: This will add Symbol.observable globally for all TypeScript users,
however, we are no longer polyfilling Symbol.observable */
declare global {
interface SymbolConstructor {
readonly observable: symbol;
}
}

Copy link
Collaborator

@cartant cartant Apr 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should revert the moving of this declaration, as its being moved solved a problem. See this PR which made the same change to the 6.x branch and the issue that it references: #6178

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cherrypicked specific PR ed34f00

@benlesh benlesh closed this Apr 15, 2021
@benlesh benlesh reopened this Apr 15, 2021
Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwonoj if you can move what you have back into types.ts I think this is GTG.

@benlesh benlesh merged commit 3c49429 into master Apr 15, 2021
@kwonoj kwonoj deleted the symbol-conflict branch August 13, 2021 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7.0.0-beta.9 conflicts symbol.observable to redux (and other have old symbol-observable version)
3 participants